Skip to content

Conversation

@EFanZh
Copy link
Contributor

@EFanZh EFanZh commented Nov 22, 2025

This PR adds hints that the return value of T::ilog10 will never exceed T::MAX.ilog10().

This works because ilog10 is a monotonically nondecreasing function, the maximum return value is reached at the max input value.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Nov 22, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 22, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@Noratrieb
Copy link
Member

Is this motivated by concrete codegen improvements (the codegen tests are quite artificial)? assume/assert_unchecked always has the potential to go either way wrt performance, so it would be good to know whether you've confirmed that this improves real-world code

@jhorstmann
Copy link
Contributor

Interesting that llvm can already infer some bounds, but apparently not the tightest ones. Looking at the following functions in compiler explorer, there are no panics:

pub fn test_u8(i: std::num::NonZeroU8, a: &[u32; 3]) -> u32 {
    a[i.ilog10() as usize]
}

pub fn test_u16(i: std::num::NonZeroU16, a: &[u32; 8]) -> u32 {
    a[i.ilog10() as usize]
}

pub fn test_u32(i: std::num::NonZeroU32, a: &[u32; 13]) -> u32 {
    a[i.ilog10() as usize]
}

pub fn test_u64(i: std::num::NonZeroU64, a: &[u32; 23]) -> u32 {
    a[i.ilog10() as usize]
}

These functions with tighter bounds would make for nicer codegen test.

@EFanZh
Copy link
Contributor Author

EFanZh commented Nov 23, 2025

@Noratrieb: An example would be getting a slice of digits of an integer where adding an assertion can remove bound checking code:

pub fn get_digits(mut x: u32, buffer: &mut [u8; 10]) -> &mut [u8] {
    let digits = &mut buffer[..x.checked_ilog10().unwrap_or_default() as usize + 1];

    for slot in &mut *digits {
        *slot = (x % 10) as u8;
        x /= 10;
    }

    digits
}

You can compare the the codegen results here.

Also, isqrt already have a similar assertion like this:

pub const fn isqrt(self) -> Self {
let result = crate::num::int_sqrt::$ActualT(self as $ActualT) as $SelfT;
// Inform the optimizer what the range of outputs is. If testing
// `core` crashes with no panic message and a `num::int_sqrt::u*`
// test failed, it's because your edits caused these assertions or
// the assertions in `fn isqrt` of `nonzero.rs` to become false.
//
// SAFETY: Integer square root is a monotonically nondecreasing
// function, which means that increasing the input will never
// cause the output to decrease. Thus, since the input for unsigned
// integers is bounded by `[0, <$ActualT>::MAX]`, sqrt(n) will be
// bounded by `[sqrt(0), sqrt(<$ActualT>::MAX)]`.
unsafe {
const MAX_RESULT: $SelfT = crate::num::int_sqrt::$ActualT(<$ActualT>::MAX) as $SelfT;
crate::hint::assert_unchecked(result <= MAX_RESULT);
}
result
}

I think it is reasonable that we do the same on ilog10, otherwise it is difficult to explain why is there behavior difference between isqrt and ilog10.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants